types(web3-core-helpers): fix disconnect args #3218
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes WebsocketProviderBase.disconnect arguments optional in the type declaration file, since they have a default value in the JavaScript implementation for the 2.x branch and the arguments aren't even present in the 1.x branch, and they should be optional per the MDN WebSocket.close() docs.
The correct change would be to actually remove the arguments in the 1.x branch and make them optional in the 2.x branch, but that would be an unnecessary breaking change for the 1.x branch without much added value (other than to warn users that the arguments they are passing won't do anything).
I have only tested this by manually introducing the change to the installed library in my project under
node_modules
and runningnpm run build
in my project, which uses Web3.Will open a PR with the same change for the 2.x branch if this one is accepted.
Relevant code:
https://github.com/ethereum/web3.js/blob/5c78c2b7cfa37afedfe1ac020bf912b3e85cfd8a/packages/web3-providers-ws/src/index.js#L401-L405
https://github.com/ethereum/web3.js/blob/8a3b5a7e1e07a5bd7b76a92005870c80cb96d521/packages/web3-providers/src/providers/WebsocketProvider.js#L132-L144
Checklist:
npm run test
with success and extended the tests if necessary.npm run build
and tested the resulting file fromdist
folder in a browser.